Skip to content

Comments

dcuc: fix file mounting in add_bind_mount#21

Open
m-iwanicki wants to merge 2 commits intomainfrom
fix-add-bind-mount
Open

dcuc: fix file mounting in add_bind_mount#21
m-iwanicki wants to merge 2 commits intomainfrom
fix-add-bind-mount

Conversation

@m-iwanicki
Copy link

add_bind_mount was run in subshell so changes to e.g. DOCKER_ARGS were not visible after finishing

add_bind_mount was run in subshell so changes to e.g. DOCKER_ARGS were
not visible after finishing

Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
@m-iwanicki
Copy link
Author

Not sure what's happening with tests. For some reason it changes . to *:

++ realpath ./data/work/bootsplash.bmp
+ ABS_PATH=*/data/work/bootsplash.bmp

I guess it's because $PWD or pwd returns * but I have no idea why.

@macpijan
Copy link
Contributor

macpijan commented Jan 9, 2025

Not sure what's happening with tests. For some reason it changes . to *:

It smells like it might be related to: https://github.com/Dasharo/dcu/blob/main/test/approve#L23 - maybe a bug with configuration on our side or with the approval tool?

Any ideas @philipandag ?

@philipanda
Copy link
Contributor

I guess it's because $PWD or pwd returns * but I have no idea why.

The testing environment we use allows replacing every occurrence of a given string with * in the output so that the tests work & pass despite being run on different machines, different users, in different directories etc.

++ realpath ./data/work/bootsplash.bmp
+ ABS_PATH=*/data/work/bootsplash.bmp

It will look like that only in tests to prevent the full absolute path from being compared across separate runs.
That only affects the output used for tests. Running manually will show the absolute paths. Do you need to compare absolute paths there?

This behavior has to be enabled on every test case separately:

describe "Try to replace logo in binary supporting that"
  allow_diff $ESCAPED_PWD
  approve "$cli logo ${DATA_WORK_DIR}/protectli_vault_cml_v1.2.0-rc1_vp46xx.rom  -l ${DATA_WORK_DIR}/bootsplash.bmp"

The allow_diff $ESCAPED_PWD causes every occurence of $ESCAPED_PWD to be replaced with * using sed.

@m-iwanicki
Copy link
Author

m-iwanicki commented Jan 14, 2025

@philipandag You are right, when I redirected pwd output to file (after running approve_ci manually) there was full path ending with: /dcu/test/. But I'm still unsure why it doesn't work. When I try to use the same command it works e.g.:

failed     ../dcuc logo ./data/work/protectli_vault_cml_v1.2.0-rc1_vp46xx.rom  -l ./data/work/bootsplash.bmp
$ ../dcuc logo ./data/work/protectli_vault_cml_v1.2.0-rc1_vp46xx.rom  -l ./data/work/bootsplash.bmp
Setting /bootsplash.bmp as custom logo
Success

Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
@m-iwanicki
Copy link
Author

Didn't see that dcuc uses dcu command when using CI. Fixed in ce30ac1 but as of now CI doesn't really test dcuc but dcu.

@m-iwanicki m-iwanicki requested review from macpijan and removed request for krystian-hebel August 13, 2025 08:28
@m-iwanicki m-iwanicki linked an issue Aug 13, 2025 that may be closed by this pull request
@m-iwanicki
Copy link
Author

@macpijan I added you as reviewer, as I don't think Krystian will review this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Misleading error message when passing ROM as ./coreboot.rom

3 participants